Revert "Refactor prompt behavior and add meta support"#2608
Conversation
This reverts commit edb9d5a.
marvin-context-protocol
Bot
added
the
enhancement
jlowin
added
the
ignore in release notes
WalkthroughThe changes refactor how prompt results are defined and used throughout the codebase. PromptResult is converted from a class to a type alias representing Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/fastmcp/prompts/prompt.py (1)
322-381: Acknowledge static analysis hint (optional refinement).The Ruff TRY300 hint suggests moving
return messagesto anelseblock for cleaner exception handling structure. This is a minor style preference and doesn't affect correctness. Given this is a revert PR, deferring this refinement is reasonable.If desired in a future PR:
except Exception as e: logger.exception(f"Error rendering prompt {self.name}") raise PromptError(f"Error rendering prompt {self.name}.") from e + else: + return messages - - return messagessrc/fastmcp/prompts/prompt_manager.py (1)
97-118: LGTM with minor nit.The
render_promptmethod correctly constructsGetPromptResultfrom the rendered messages and prompt description.Per static analysis hint,
raise eon line 112 can be simplified to justraisewhen re-raising the caught exception:except PromptError as e: logger.exception(f"Error rendering prompt {name!r}") - raise e + raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
AGENTS.mdis excluded by none and included by nonetests/prompts/test_prompt.pyis excluded by none and included by nonetests/prompts/test_prompt_manager.pyis excluded by none and included by nonetests/server/test_server.pyis excluded by none and included by nonetests/server/test_server_interactions.pyis excluded by none and included by none
📒 Files selected for processing (8)
docs/servers/prompts.mdx(1 hunks)src/fastmcp/prompts/__init__.py(1 hunks)src/fastmcp/prompts/prompt.py(6 hunks)src/fastmcp/prompts/prompt_manager.py(4 hunks)src/fastmcp/server/middleware/caching.py(4 hunks)src/fastmcp/server/middleware/middleware.py(2 hunks)src/fastmcp/server/proxy.py(5 hunks)src/fastmcp/server/server.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/prompts.mdx
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
Documentation files must be included in docs.json to be published
Files:
docs/servers/prompts.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: All Python code must use Python ≥ 3.10 with full type annotations
Never use bareexcept- always be specific with exception types
Files:
src/fastmcp/server/middleware/middleware.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/prompts/prompt_manager.pysrc/fastmcp/prompts/__init__.pysrc/fastmcp/server/server.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/middleware/caching.py
🧠 Learnings (2)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions
Applied to files:
src/fastmcp/prompts/prompt_manager.pysrc/fastmcp/server/proxy.py
📚 Learning: 2025-12-13T17:09:13.085Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-13T17:09:13.085Z
Learning: Applies to tests/**/*.py : Pass FastMCP servers directly to clients for testing (in-memory transport), only use HTTP transport when explicitly testing network features
Applied to files:
src/fastmcp/server/proxy.py
🧬 Code graph analysis (2)
src/fastmcp/server/server.py (1)
src/fastmcp/prompts/prompt.py (1)
FunctionPrompt(158-381)
src/fastmcp/server/middleware/caching.py (2)
src/fastmcp/prompts/prompt.py (1)
Prompt(64-155)src/fastmcp/server/middleware/middleware.py (1)
CallNext(42-43)
🪛 Ruff (0.14.8)
src/fastmcp/prompts/prompt.py
378-378: Consider moving this statement to an else block
(TRY300)
src/fastmcp/prompts/prompt_manager.py
112-112: Use raise without specifying exception name
Remove exception name
(TRY201)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (16)
src/fastmcp/prompts/__init__.py (1)
1-9: LGTM - Clean public API surface.The removal of
PromptResultfrom the public exports is consistent with this revert. The type alias remains available inprompt.pyfor internal use and direct imports when needed, but is not part of the promoted public API from this package.src/fastmcp/prompts/prompt.py (1)
43-49: LGTM - Well-structured type aliases.The
SyncPromptResultandPromptResulttype aliases clearly define the acceptable return types from prompt functions. The union type correctly captures strings, PromptMessage objects, dicts, sequences, and their awaitable variants.src/fastmcp/server/middleware/middleware.py (2)
20-20: LGTM - Import cleanup.Removing the
PromptResultimport aligns with the shift to usingmt.GetPromptResultfrom the MCP types package for the prompt retrieval pathway.
170-175: LGTM - Consistent use of MCP protocol types.The
on_get_promptmethod now correctly usesmt.GetPromptResultas the return type, which is the canonical MCP protocol type forprompts/getresponses. This aligns with how other middleware hooks usemt.*types and ensures consistency with the caching middleware.src/fastmcp/server/middleware/caching.py (3)
20-20: LGTM - Import updated consistently.The import change removes
PromptResultin favor of usingmcp.types.GetPromptResultdirectly, consistent with the middleware base class changes.
223-229: LGTM - Cache adapter typed correctly.The
_get_prompt_cachenow usesmcp.types.GetPromptResultas its Pydantic model, which is appropriate since this MCP type is already a well-defined Pydantic model suitable for serialization without needing a customCachable*wrapper.
418-444: LGTM - on_get_prompt implementation aligned with base class.The method signature and internal typing correctly use
mcp.types.GetPromptResultthroughout, matching the baseMiddlewareclass. The caching logic remains unchanged - just the types are updated for consistency.docs/servers/prompts.mdx (1)
201-210: This import pattern is correct and intentional.PromptResultis an internal type alias for return type annotations and is not part of the public API—its absence from__all__is by design, not an oversight. The example imports it directly fromfastmcp.prompts.prompt, which is the proper way to access it for type hints.The submodule import pattern shown here is also consistent with other examples in the same documentation file (line 31), and both
MessageandPromptResultare appropriately imported from the location where they're defined. No changes are needed.src/fastmcp/server/proxy.py (4)
19-21: LGTM!Import of
GetPromptResultfrommcp.typesis correctly added to support the updated return type inProxyPromptManager.render_prompt.
31-31: LGTM!Import of
PromptMessageadded to support thelist[PromptMessage]return type inProxyPrompt.render.
256-270: LGTM!The
render_promptmethod correctly returnsGetPromptResultdirectly from the proxy client call, aligning with the parentPromptManager.render_promptsignature and the mcp library's type system.
527-531: LGTM!The
ProxyPrompt.rendermethod correctly extracts and returnsresult.messagesaslist[PromptMessage], which aligns with the basePrompt.rendermethod's expected return type.src/fastmcp/server/server.py (2)
63-63: LGTM!Import simplified to only include
FunctionPrompt, which is the only type used fromfastmcp.prompts.promptin this file.
1760-1798: LGTM!The
_get_promptmethod correctly returnsGetPromptResultand properly delegates to_get_prompt_middlewarefor mounted servers, maintaining consistency with the middleware chain. The return type aligns with_get_prompt_mcp(line 1719) and_get_prompt_middleware(line 1744).src/fastmcp/prompts/prompt_manager.py (2)
7-11: LGTM!Imports correctly updated to include
GetPromptResultfrom mcp andFunctionPrompt,Prompt,PromptResultfrom the local prompt module.
59-65: LGTM!Type annotation for the
fnparameter improves code clarity and IDE support.
| if mask_error_details is None | ||
| else mask_error_details | ||
| ) | ||
| self.mask_error_details = mask_error_details or settings.mask_error_details |
There was a problem hiding this comment.
Bug: mask_error_details=False will be ignored.
Using or operator means an explicit False value will be overridden by settings.mask_error_details. If a caller passes mask_error_details=False, the settings default will be used instead.
Apply this diff to preserve explicit False values:
- self.mask_error_details = mask_error_details or settings.mask_error_details
+ self.mask_error_details = mask_error_details if mask_error_details is not None else settings.mask_error_details📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.mask_error_details = mask_error_details or settings.mask_error_details | |
| self.mask_error_details = mask_error_details if mask_error_details is not None else settings.mask_error_details |
🤖 Prompt for AI Agents
In src/fastmcp/prompts/prompt_manager.py around line 27, the assignment
self.mask_error_details = mask_error_details or settings.mask_error_details
ignores an explicit False passed by callers; change the assignment to preserve
explicit False by using a None check (e.g. self.mask_error_details =
mask_error_details if mask_error_details is not None else
settings.mask_error_details) so that only None falls back to settings.
1 participant
Reverts #2600
There are sufficient changes coming, including this, that I want to keep it as part of a holistic internal refactor in 2.15.0. Reverting so we can cleanly release 2.14.1 first.